Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

arm64: fix fvp smp faild to boot #14476

Merged
merged 1 commit into from
Oct 24, 2024
Merged

arm64: fix fvp smp faild to boot #14476

merged 1 commit into from
Oct 24, 2024

Conversation

hujun260
Copy link
Contributor

Summary

fix fvp smp faild to boot
we should give a busy wait addr
This commit fixes the regression from #13640

Impact

arm64 fvp smp

Testing

fvp-armv8r:nsh_smp ostest

@hujun260 hujun260 mentioned this pull request Oct 23, 2024
Closed
1 task
@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: XS The size of the change in this PR is very small labels Oct 23, 2024
@xiaoxiang781216 xiaoxiang781216 linked an issue Oct 23, 2024 that may be closed by this pull request
[HELP] ARMv8r FVP (fvp-armv8r) does not shows boot up logs and telnet to UART 1 port does not prompt to nuttx shell #13765
Closed
1 task
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: XS The size of the change in this PR is very small labels Oct 23, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 23, 2024

[Experimental Bot, please feedback here]

This pull request appears to meet the basic NuttX requirements. However, it could be improved with some clarifications:

Summary:

  • More Detail: Briefly explain why the busy wait address is needed. What was the specific issue causing the FVP SMP boot failure?
  • Functional Change: Be specific about which part of the code is modified. For example, "Added a busy wait loop in the ARM64 FVP platform initialization code".
  • How it Works: Explain at a high level how providing the busy wait address resolves the boot issue.

Impact:

  • Hardware: While you mention "arm64 fvp smp", be explicit about the specific architectures and boards affected.
  • Documentation: If this fix impacts user understanding (e.g., if the busy wait address is configurable), mention if a documentation update is needed or provided.

Testing:

  • Build Host: Specify the operating system, CPU architecture, and compiler used for testing.
  • Target(s): Provide the full architecture and board configuration used (e.g., "arm64, FVP:nsh_smp").
  • Logs: While "ostest" is mentioned, consider including relevant snippets of the logs that demonstrate the issue before the fix and the successful outcome after the fix.

By adding these details, you'll make it easier for reviewers to understand and evaluate your pull request.

XuNeo
XuNeo reviewed Oct 23, 2024
View reviewed changes
boards/arm64/fvp-v8r/fvp-armv8r/configs/nsh_smp/defconfig
@@ -14,6 +14,8 @@ CONFIG_ARCH_CHIP_FVP_ARMV8R=y
CONFIG_ARCH_CHIP_FVP_R82=y
CONFIG_ARCH_EARLY_PRINT=y
CONFIG_ARCH_INTERRUPTSTACK=4096
CONFIG_ARM64_SMP_BUSY_WAIT=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why FVP needs busy wait?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FVP SMP startup has always been using the busywait strategy. Before my modification, it serially waited for a fixed global variable (cpu_ready_flag). After I changed it to parallel startup for CPUs (1 to n), I added a wait address for more generality. However, at that time, the FVP SMP was not configured accordingly, leading to this error.

Additionally, let me mention that the arm64 QEMU SMP startup uses the PSCI method, rather than busywait.

@hujun260
arm64: fix fvp smp faild to boot
00ef274 
reason:
we should give a busy wait addr

This commit fixes the regression from apache#13640

Signed-off-by: hujun5 <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit 9395669 into apache:master Oct 24, 2024
12 checks passed
@jerpelea jerpelea mentioned this pull request Oct 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuNeo XuNeo XuNeo left review comments

@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

@anchao anchao anchao approved these changes

Assignees
No one assigned
Labels
Arch: arm64 Issues related to ARM64 (64-bit) architecture Board: arm64 Size: S The size of the change in this PR is small
Projects
None yet
Milestone
No milestone
5 participants
@hujun260 @nuttxpr @anchao @xiaoxiang781216 @XuNeo